Pull Request Test Coverage Report for Build 15148305686

Details

• 3 of 3 (100.0%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.005%) to 86.075%

Totals
Change from base Build 15144463728:	0.005%
Covered Lines:	7269
Relevant Lines:	8445

💛 - Coveralls

Side note: I think check_fee! should just return the bitcoin Amount unwrapped instead of being forced to deal with the Option everywhere. If we fail to calculate the fee it should probably just panic to indicate a failing test.

Should I fix this clippy warning? It's unrelated.

error: use of deprecated method `bitcoin::key::TweakedKeypair::to_inner`: use to_keypair() instead
   --> wallet/src/wallet/signer.rs:580:14
    |
580 |             .to_inner(),
    |              ^^^^^^^^
    |
    = note: `-D deprecated` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(deprecated)]`

I would say go ahead and change TweakedKeypair::to_inner to use to_keypair instead. For context see rust-bitcoin/rust-bitcoin#4450.

@ValuedMammal fixed.

It looks good overall, tests are passing for me.

@ValuedMammal fixed.

When I updated to a5d1338, the tests failed locally because the compiler was using bitcoin version 0.32.5.

--> wallet/src/wallet/signer.rs:580:14
    |
578 |           None => keypair
    |  _________________-
579 | |             .tap_tweak(secp, psbt_input.tap_merkle_root)
580 | |             .to_keypair(),
    | |             -^^^^^^^^^^ method not found in `TweakedKeypair`
    | |_____________|
    |
    

I temporarily updated the version to 0.32.6, and after that, the compiler used the new version, and the tests passed. I'm uncertain if it's ideal to upgrade to 0.32.6, and I'm curious if anyone else has experienced the same issue.

In a1d58c3: fix: clippy

Just noting that I don't think this is actually needed until we update the rust-version of the repo to rust 1.86. But I don't see a problem with including the change anyway.

Just noting that I don't think this is actually needed until we update the rust-version of the repo to rust 1.86. But I don't see a problem with including the change anyway.

Yes, no harm in just preemptively adding this though.

I temporarily updated the version to 0.32.6, and after that, the compiler used the new version, and the tests passed. I'm uncertain if it's ideal to upgrade to 0.32.6, and I'm curious if anyone else has experienced the same issue.

Yeah, my linter and CI were complaining so I updated it.

tACK a5d1338

nit: Considering how large the tests/wallet.rs file is, will it be ideal to someday organize the test cases into modules based on functionality? something like persistence, tx creation, etc

Rebased to trigger CI.